Skip to content

Comments

RCF-1277 RCF-1378 Added audit logs#681

Open
MadhuMosip wants to merge 5 commits intomosip:developfrom
MadhuMosip:RCF-1277-A
Open

RCF-1277 RCF-1378 Added audit logs#681
MadhuMosip wants to merge 5 commits intomosip:developfrom
MadhuMosip:RCF-1277-A

Conversation

@MadhuMosip
Copy link
Contributor

@MadhuMosip MadhuMosip commented Feb 10, 2026

Summary by CodeRabbit

  • Chores
    • Expanded audit logging across authentication, biometrics, demographics, packet lifecycle, sync and validation flows to improve operational tracking.
    • Added many new audit event types for user, packet, sync, registration and biometric actions.
    • Integrated audit emissions into UI flows (biometric capture, operator authentication, acknowledgement printing, packet approval/rejection).
    • Propagated audit manager into additional services and providers to ensure consistent audit generation.

Signed-off-by: Madhuravas reddy <madhu@mosip.io>
@coderabbitai
Copy link

coderabbitai bot commented Feb 10, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds extensive audit instrumentation: new AuditEvent constants; AuditManagerService propagation into DI and multiple service constructors; audit calls inserted across API, service, batch, and UI layers; AuditManagerServiceImpl audit construction now guarded. No public API removals or signature reductions.

Changes

Cohort / File(s) Summary
API Services
android/app/src/main/java/io/mosip/registration_client/api_services/...
AuthenticationApi.java, BiometricsDetailsApi.java, DemographicsDetailsApi.java, MachineDetailsApi.java, MasterDataSyncApi.java, PacketAuthenticationApi.java, UserDetailsApi.java
Inserted auditManagerService.audit / auditWithArguments calls at method entries and key state changes (e.g., FETCH_USR_ROLE, REG_BIO_, SAVE_DETAIL_TO_DTO, FETCH_CNTR_NAME, SYNCJOB_INFO_FETCH, PACKET_). No public signatures changed.
Batch & Utils
android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java
Added audit calls for sync/upload lifecycle (PACKET_SYNCED_TO_SERVER, PACKET_UPLOAD, PACKET_UPLOADED, PACKET_INTERNAL_ERROR).
DI / Module Wiring
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/config/AppModule.java
Provider methods updated to accept AuditManagerService and pass it into constructed services (MasterDataServiceImpl, PacketServiceImpl, PreCheckValidatorServiceImpl). Public provider signatures changed accordingly.
AuditEvent Constants
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java
Added numerous new AuditEvent enum constants across USER, PACKET, SYNC, REGISTRATION and BIOMETRICS categories (many new literals and grouping comments).
Service Implementations
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/...
PacketServiceImpl.java, PreCheckValidatorServiceImpl.java, MasterDataServiceImpl.java, RegistrationServiceImpl.java, AuditManagerServiceImpl.java
Constructors extended to accept AuditManagerService; new private auditManagerService fields; added audit calls in validation, packet creation/encryption/zip, sync validations, packet limit checks; AuditManagerServiceImpl refactored to guard audit construction/insertion and derive host/session metadata.
Packet Service (clientmanager)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java
Added audit call in isMaxPacketCountLimitReached and introduced auditManagerService dependency via constructor.
Pre-check Validator
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PreCheckValidatorServiceImpl.java
Added audit calls for sync/status and geo validations; constructor extended to accept auditManagerService.
UI (Flutter)
lib/ui/...
approve_packet/widget/template_bottom_sheet.dart, post_registration/acknowledgement_page.dart, process_ui/generic_process.dart, process_ui/widgets_mobile/biometric_capture_scan_block_portrait.dart
Updated audit event identifiers and added audit log calls for operator auth, acknowledgement printing/preview navigation, biometric "Next" action (one handler converted to async).

Sequence Diagram(s)

(Skipped — changes are instrumentation/wiring and do not introduce a new multi-component sequential feature requiring visualization.)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • RCF-1277 Added ARC Audit #667: Overlapping audit-logging additions and AuditEvent enum changes across many of the same APIs and service implementations.

Suggested reviewers

  • ase-101

Poem

"🐰 I hopped through code with quiet cheer,

Tucked tiny logs both far and near.
Packets, scans, and centers told,
New events written, bright and bold.
A carrot for each audit here."

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the primary objective of the changeset: adding audit logging functionality across multiple API service classes and utility files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 Fix all issues with AI agents
In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`:
- Around line 243-244: The audit call for SYNC_DEVICE_DETAILS in
MasterDataSyncApi.syncMasterData() is misleading because this method does not
sync device details; remove the
auditManagerService.audit(AuditEvent.SYNC_DEVICE_DETAILS,
Components.REGISTRATION) invocation or wrap it in a conditional that only runs
when device details are actually synced (e.g., after a device-sync code path or
when a DeviceDetails result/flag is present). Update only the audit call
(located next to auditManagerService.audit(AuditEvent.SYNC_MASTER_DATA,
Components.REGISTRATION)) so the logged events accurately reflect the work done
and remain consistent with the masterSyncJob path.

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java`:
- Line 119: The audit events PACKET_SYNCED_TO_SERVER and PACKET_UPLOADED are
being recorded with Components.REGISTRATION in PacketAuthenticationApi (via
auditManagerService.audit(...)), but BatchJob.java emits the same events with
Components.REG_PACKET_LIST; make them consistent by updating the
auditManagerService.audit calls in PacketAuthenticationApi for
AuditEvent.PACKET_SYNCED_TO_SERVER and AuditEvent.PACKET_UPLOADED to use
Components.REG_PACKET_LIST (so both PacketAuthenticationApi and BatchJob.java
attribute the same component).
- Line 219: Update the misspelled audit enum and its usages: rename the
AuditEvent constant PACKET_RETRIVE to PACKET_RETRIEVE in the AuditEvent enum
(and correct its enum description text from "retrived" to "retrieved"), then
update all references such as the call
auditManagerService.audit(AuditEvent.PACKET_RETRIVE, Components.REGISTRATION) in
PacketAuthenticationApi to use AuditEvent.PACKET_RETRIEVE so compilation and
logs reflect the correct spelling.

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/UserDetailsApi.java`:
- Line 72: The audit event AuditEvent.VALIDATE_USER_CRED is emitted in
validateUser immediately after calling isValidUserId even though no credential
validation occurs; update the audit to accurately reflect the action by either
replacing AuditEvent.VALIDATE_USER_CRED with a more appropriate event like
AuditEvent.VALIDATE_USER_ID (or ADD/CREATE a new enum constant) inside the
validateUser flow, or move the auditManagerService.audit(...) call to the point
where credentials are actually checked—identify and modify the
auditManagerService.audit(...) invocation and the AuditEvent enum to keep event
names consistent with the actual operation.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java`:
- Around line 175-179: The AuditEvent enum contains duplicate audit codes:
PACKET_CREATION_SUCCESS, PACKET_ENCRYPTED, PACKET_UPLOADED,
PACKET_SYNCED_TO_SERVER (using REG-EVT-066..069) and REG_BIO_LEFT_SLAP_SCAN /
REG_BIO_SCAN share REG-EVT-030; update these enum constants to use unique,
non-colliding codes (e.g., choose the next unused REG-EVT numbers) in
AuditEvent, confirm no other constants (like APPR_* or NEXT_BUTTON_CLICKED) use
those codes, and adjust any downstream consumers/tests/configs that reference
the old values so analytics and correlation remain correct. Ensure you search
the repo for the old codes before changing and keep the enum names unchanged to
avoid API breaks.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java`:
- Line 428: The audit call
auditManagerService.audit(AuditEvent.SYNC_PKT_COUNT_VALIDATE,
Components.REGISTRATION) is invoked before the packet count validation and is
executed inside the same try that can short-circuit validation; move this audit
invocation to after the actual validation logic (so it only logs when validation
ran) and wrap the audit call in its own small try-catch that logs audit failures
but does not rethrow, ensuring packet count validation in the PacketServiceImpl
method always runs even if audit() fails.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PreCheckValidatorServiceImpl.java`:
- Line 84: The call to auditManagerService.audit(...) in
PreCheckValidatorServiceImpl may throw and is currently allowed to bubble up and
be re-wrapped as a ClientCheckedException which can incorrectly block
registration; modify the code around the audit call
(auditManagerService.audit(AuditEvent.SYNC_INFO_VALIDATE,
Components.JOB_SERVICE)) to execute it inside its own try-catch, catch and log
any exception (or use auditManagerService-specific error handling) but do not
rethrow or translate it into OPT_TO_REG_TIME_SYNC_EXCEED — treat it as
fire-and-forget so the rest of the validation flow continues even if audit
fails.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java`:
- Around line 308-313: In RegistrationServiceImpl adjust the flow so
auditManagerService.audit(AuditEvent.PACKET_ENCRYPTED, ...) and
AuditEvent.PACKET_INTERNAL_ZIP are only invoked after persistPacket returns a
non-null result: move or wrap those audit calls to occur after the persistPacket
null-check (i.e., after verifying the returned packet != null). Also fix the
containerPath null-check by changing the condition from "containerPath != null
|| !containerPath.trim().isEmpty()" to use logical AND ("containerPath != null
&& !containerPath.trim().isEmpty()") so it doesn't NPE when containerPath is
null.
🧹 Nitpick comments (2)
android/app/src/main/java/io/mosip/registration_client/api_services/DemographicsDetailsApi.java (1)

49-49: All four demographic mutation methods use the same SAVE_DETAIL_TO_DTO event.

Consider whether the audit trail needs to distinguish between addDemographicField, addSimpleTypeDemographicField, setDateField, and setConsentField. Currently, all four emit the identical event, so any downstream audit analysis cannot differentiate the operation type. If this is intentional (coarse-grained auditing for demographics), no action is needed.

Also applies to: 87-87, 123-123, 142-142

android/app/src/main/java/io/mosip/registration_client/api_services/UserDetailsApi.java (1)

51-52: USER_STATUS_FETCH and FETCH_USR_DET are logged before any input validation.

Both events will be recorded even when username is empty/null and the method returns early with REG_USER_EMPTY. This is fine if the intent is to track all validation attempts, but if you only want to audit successful lookups, move these after the input checks.

Signed-off-by: Madhuravas reddy <madhu@mosip.io>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`:
- Line 438: The unguarded call to
auditManagerService.audit(AuditEvent.SYNCJOB_INFO_FETCH, Components.JOB_SERVICE)
inside getActiveSyncJobs can throw and block returning results; wrap that audit
invocation in its own try-catch so any exceptions are swallowed or logged (do
not rethrow) to preserve the method's normal return path. Specifically, locate
the audit call in getActiveSyncJobs, surround auditManagerService.audit(...)
with try { ... } catch (Exception e) { /* log audit failure via logger or
auditManagerService.log/error but do not throw */ }, and ensure the catch only
records the failure without impacting the method's result.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/AuditEvent.java`:
- Around line 176-179: Fix the typos in the audit event description strings for
PACKET_CREATION_SUCCESS, PACKET_ENCRYPTED, and PACKET_SYNCED_TO_SERVER in the
AuditEvent enum: change "Packet Succesfully Created", "Packet Encrypted
Sucessfully", and "Packet Synced to Server Sucesfully" to use the correct
spelling "Successfully" (e.g., "Packet Successfully Created", "Packet Encrypted
Successfully", "Packet Synced to Server Successfully") so persisted audit logs
use the corrected text.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PreCheckValidatorServiceImpl.java`:
- Around line 296-297: The audit call
auditManagerService.audit(AuditEvent.SYNC_GEO_VALIDATE, Components.REGISTRATION)
in validateCenterToMachineDistance is unguarded and can throw
non-NumberFormatException errors that will block registration; wrap that single
audit call in its own try-catch that catches Exception (or Throwable) and logs
the failure via the existing logger (e.g., logger.error(..., e)) without
rethrowing, mirroring the handling used for SYNC_INFO_VALIDATE so the audit
remains fire-and-forget.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java`:
- Around line 342-344: persistPacket can return a null/empty containerPath but
the code still calls insertRegistration and emits
auditManagerService.audit(AuditEvent.PACKET_CREATION_SUCCESS,
Components.REGISTRATION); — change the flow in RegistrationServiceImpl so you
check the result of persistPacket (containerPath) immediately: if containerPath
is null/empty, log/audit a failure and return or throw (so insertRegistration is
not called), otherwise proceed to call insertRegistration and then emit
PACKET_CREATION_SUCCESS; reference the persistPacket call site,
insertRegistration(...) invocation, and the auditManagerService.audit(...) call
when making the change.
🧹 Nitpick comments (1)
android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java (1)

194-197: Inconsistent Components value in upload error path.

PACKET_INTERNAL_ERROR uses Components.REGISTRATION while all other audit calls in uploadPacketAll use Components.REG_PACKET_LIST. This will make error events harder to correlate with the corresponding upload flow in audit queries.

-                auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR, Components.REGISTRATION);
+                auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR, Components.REG_PACKET_LIST);

Signed-off-by: Madhuravas reddy <madhu@mosip.io>
Copy link
Collaborator

@ase-101 ase-101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check comments

Signed-off-by: Madhuravas reddy <madhu@mosip.io>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/AuditManagerServiceImpl.java (1)

169-186: Optimize: capture System.currentTimeMillis() once instead of calling it twice.

The constructor call invokes System.currentTimeMillis() at lines 170 and 174, producing two slightly different timestamp values for what represents the same event instant. Store it in a local variable long now = System.currentTimeMillis(); and reuse it for both createdAt and actionTimeStamp.

Suggested: capture timestamp once
+        long now = System.currentTimeMillis();
         Audit audit = new Audit(
-                System.currentTimeMillis(),
+                now,
                 auditEventEnum.getId(),
                 auditEventEnum.getName(),
                 auditEventEnum.getType(),
-                System.currentTimeMillis(),
+                now,
                 hostName,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/AuditManagerServiceImpl.java`
around lines 169 - 186, The Audit constructor is called with two separate
System.currentTimeMillis() invocations in AuditManagerServiceImpl which can
yield slightly different timestamps; capture a single timestamp into a local
long variable (e.g., long now = System.currentTimeMillis();) before creating the
Audit and reuse that variable for both the createdAt and actionTimeStamp
parameters when constructing the Audit object.
android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/MasterDataServiceImpl.java (1)

545-545: Wrap audit call defensively to avoid aborting syncUserDetails on audit failure.

If auditManagerService.audit() throws unexpectedly, syncUserDetails will propagate the exception and skip the actual user-details sync. The same concern was addressed in PacketServiceImpl.isMaxPacketCountLimitReached() in a prior review iteration. For consistency, wrap this in a try-catch.

Proposed fix
     public void syncUserDetails(Runnable onFinish, boolean isManualSync, String jobId) throws Exception {
-        auditManagerService.audit(AuditEvent.FETCH_USR_DET, Components.LOGIN);
+        try {
+            auditManagerService.audit(AuditEvent.FETCH_USR_DET, Components.LOGIN);
+        } catch (Exception e) {
+            Log.e(TAG, "Audit logging failed", e);
+        }
         String serverVersion = getServerVersionFromConfigs();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/MasterDataServiceImpl.java`
at line 545, The call to auditManagerService.audit(AuditEvent.FETCH_USR_DET,
Components.LOGIN) inside syncUserDetails should be wrapped defensively so an
unexpected exception from auditManagerService does not abort the sync; modify
syncUserDetails to call auditManagerService.audit(...) inside a try-catch that
logs the audit failure (using existing logger or auditManagerService if
appropriate) and swallows the exception so processing continues—mirror the safe
pattern used in PacketServiceImpl.isMaxPacketCountLimitReached() to locate the
intended handling style.
android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java (1)

192-196: Audit call inside catch block could throw and mask error handling.

If auditManagerService.audit() on line 195 throws, the exception escapes the catch block and prevents any remaining iterations of the upload loop from processing. Wrap the audit call defensively.

Proposed fix
             } catch (Exception e) {
                 syncAndUploadInProgressStatus = false;
                 Log.e(getClass().getSimpleName(), e.getMessage());
-                auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR, Components.REG_PACKET_LIST, e.getMessage());
+                try {
+                    auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR, Components.REG_PACKET_LIST, e.getMessage());
+                } catch (Exception auditEx) {
+                    Log.e(getClass().getSimpleName(), "Audit logging failed", auditEx);
+                }
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java`
around lines 192 - 196, The catch block around the upload loop currently calls
auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR,
Components.REG_PACKET_LIST, e.getMessage()) which itself may throw and escape
the catch; wrap that single auditManagerService.audit(...) call in its own
try-catch so any exception from audit is caught and logged (e.g., with Log.e
using getClass().getSimpleName()) and not rethrown, preserving
syncAndUploadInProgressStatus = false and allowing the upload loop to continue;
keep the original logging of the primary exception (Log.e(..., e.getMessage()))
and ensure the audit fallback catch does not alter control flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java`:
- Line 196: The audit call using AuditEvent.PACKET_INTERNAL_ERROR in
PacketAuthenticationApi (auditManagerService.audit(...)) currently passes
Components.REGISTRATION but should use Components.REG_PACKET_LIST to match
BatchJob's usage and keep audit component consistent; update the
auditManagerService.audit invocation that references
AuditEvent.PACKET_INTERNAL_ERROR to pass Components.REG_PACKET_LIST instead of
Components.REGISTRATION.

---

Duplicate comments:
In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/PacketServiceImpl.java`:
- Around line 442-447: The audit call can throw and currently lives inside the
outer try/catch that returns false on any Exception, which can override the
computed result (limitReached); update the method in PacketServiceImpl so that
after computing boolean limitReached = yetToExportCount >= maxCount you invoke
auditManagerService.audit(AuditEvent.SYNC_PKT_COUNT_VALIDATE,
Components.REGISTRATION) inside its own try/catch that swallows/logs audit
exceptions (do not change limitReached), ensuring any exception from audit does
not change the method's returned value.

---

Nitpick comments:
In `@android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java`:
- Around line 192-196: The catch block around the upload loop currently calls
auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR,
Components.REG_PACKET_LIST, e.getMessage()) which itself may throw and escape
the catch; wrap that single auditManagerService.audit(...) call in its own
try-catch so any exception from audit is caught and logged (e.g., with Log.e
using getClass().getSimpleName()) and not rethrown, preserving
syncAndUploadInProgressStatus = false and allowing the upload loop to continue;
keep the original logging of the primary exception (Log.e(..., e.getMessage()))
and ensure the audit fallback catch does not alter control flow.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/AuditManagerServiceImpl.java`:
- Around line 169-186: The Audit constructor is called with two separate
System.currentTimeMillis() invocations in AuditManagerServiceImpl which can
yield slightly different timestamps; capture a single timestamp into a local
long variable (e.g., long now = System.currentTimeMillis();) before creating the
Audit and reuse that variable for both the createdAt and actionTimeStamp
parameters when constructing the Audit object.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/MasterDataServiceImpl.java`:
- Line 545: The call to auditManagerService.audit(AuditEvent.FETCH_USR_DET,
Components.LOGIN) inside syncUserDetails should be wrapped defensively so an
unexpected exception from auditManagerService does not abort the sync; modify
syncUserDetails to call auditManagerService.audit(...) inside a try-catch that
logs the audit failure (using existing logger or auditManagerService if
appropriate) and swallows the exception so processing continues—mirror the safe
pattern used in PacketServiceImpl.isMaxPacketCountLimitReached() to locate the
intended handling style.

Signed-off-by: Madhuravas reddy <madhu@mosip.io>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java (1)

237-238: packetId is absent from auditWithArguments calls, reducing per-packet traceability.

Both updatePacketStatus and supervisorReview have the packet identifier available (packetId) but pass only the component ID/name and the status string. Without the packet ID, reconstructing a per-packet audit trail from these events is not possible.

♻️ Proposed improvement — include packetId
 // updatePacketStatus
-        auditManagerService.auditWithArguments(AuditEvent.PACKET_STATUS_UPDATE,
-                Components.REGISTRATION.getId(), Components.REGISTRATION.getName(), clientStatus);
+        auditManagerService.auditWithArguments(AuditEvent.PACKET_STATUS_UPDATE,
+                Components.REGISTRATION.getId(), Components.REGISTRATION.getName(), packetId, clientStatus);

 // supervisorReview
-        auditManagerService.auditWithArguments(AuditEvent.PACKET_STATUS_UPDATE,
-                Components.REGISTRATION.getId(), Components.REGISTRATION.getName(), supervisorStatus);
+        auditManagerService.auditWithArguments(AuditEvent.PACKET_STATUS_UPDATE,
+                Components.REGISTRATION.getId(), Components.REGISTRATION.getName(), packetId, supervisorStatus);

The auditWithArguments method accepts variable String arguments via varargs, so passing both the packet ID and status is straightforward.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java`
around lines 237 - 238, The audit call is missing the packet identifier—update
the auditManagerService.auditWithArguments invocation(s) (e.g., the call using
AuditEvent.PACKET_STATUS_UPDATE with Components.REGISTRATION.getId()/getName()
and clientStatus) to include packetId as an argument (for example, pass packetId
and then clientStatus) so the per-packet audit trail can be reconstructed; make
the same change in other similar calls such as from
supervisorReview/updatePacketStatus that call auditWithArguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java`:
- Line 196: The audit call using
auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR,
Components.REG_PACKET_LIST, e.getMessage()) may pass a null message when
Exception e has no message; change the third argument to a null-safe string
(e.g., use e.getMessage() if non-null else e.toString() or a fixed fallback like
"Exception occurred") so auditManagerService.audit always receives a non-null,
meaningful message; update the call site in PacketAuthenticationApi where
auditManagerService.audit is invoked and keep the same
AuditEvent.PACKET_INTERNAL_ERROR and Components.REG_PACKET_LIST parameters.

---

Nitpick comments:
In
`@android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java`:
- Around line 237-238: The audit call is missing the packet identifier—update
the auditManagerService.auditWithArguments invocation(s) (e.g., the call using
AuditEvent.PACKET_STATUS_UPDATE with Components.REGISTRATION.getId()/getName()
and clientStatus) to include packetId as an argument (for example, pass packetId
and then clientStatus) so the per-packet audit trail can be reconstructed; make
the same change in other similar calls such as from
supervisorReview/updatePacketStatus that call auditWithArguments.

});
} catch (Exception e) {
Log.e(getClass().getSimpleName(), e.getMessage());
auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR, Components.REG_PACKET_LIST, e.getMessage());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

e.getMessage() can return null, silently passing null to the audit method.

Exception.getMessage() returns null when the exception was constructed without a message (e.g., new IOException(cause)). Depending on the audit(AuditEvent, Components, String) implementation, this could cause an NPE or emit a meaningless "null" string into the audit log.

🛡️ Proposed null-safe fix
-                auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR, Components.REG_PACKET_LIST, e.getMessage());
+                auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR, Components.REG_PACKET_LIST,
+                        e.getMessage() != null ? e.getMessage() : e.getClass().getSimpleName());

Also, the previously flagged inconsistency on this line (PACKET_INTERNAL_ERROR using Components.REGISTRATION instead of Components.REG_PACKET_LIST) has been corrected in the current code — that past review concern is now resolved. ✅

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/PacketAuthenticationApi.java`
at line 196, The audit call using
auditManagerService.audit(AuditEvent.PACKET_INTERNAL_ERROR,
Components.REG_PACKET_LIST, e.getMessage()) may pass a null message when
Exception e has no message; change the third argument to a null-safe string
(e.g., use e.getMessage() if non-null else e.toString() or a fixed fallback like
"Exception occurred") so auditManagerService.audit always receives a non-null,
meaningful message; update the call site in PacketAuthenticationApi where
auditManagerService.audit is invoked and keep the same
AuditEvent.PACKET_INTERNAL_ERROR and Components.REG_PACKET_LIST parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants